Thanks for the report.

The "reload" keyword wasn't really meant to be used in a call to connect(). AsyncSSH should already be rereading the configuration file for each new connect() call, with updated host/port/username information each time. Reload is meant to be used when you have an already populated SSHClientConnectionOptions object and you need re-evaluate the config after updating one of the matchable inputs, like when doing hostname canonicalization.

Can you provide an example where it fails without this change? I tried connecting through multiple jump hosts where I changed the host, port, and username and it got the right values on each hop, even without this.

You are correct.
I've updated the unit test using a dedicated jump host service - it works.
I'll keep this open, trying to reproduce.

Thanks

Now it reproduces.

Host jump
        HostName 127.0.0.3
        Port {jump_port}
        User jumper

Host target
        Hostname 127.0.0.2
        Port {self._server_port}

Match final host 127.0.0.2
        ProxyJump jump

   ConnectionRefusedError: [Errno 111] Connect call failed ('127.0.0.3', 22)

And my fix does not.

Anyway - is there a way to run a single unit tests?
I struggle due to the relative imports in tests/ without a package ("ImportError: attempted relative import with no known parent package")

With your latest commit, you are setting reload=False, which should be the same thing as the original code, since False is the default. So, the only code left in the commit is basically the unit test.

Regarding running a single unit test, you can specify something like:

python3 -m unittest tests.test_forward._TestTCPForwarding.test_proxy_jump_user

Do this from the parent directory (with "asyncssh" and "tests" subdirectories). This should avoid the relative import complaints.

The error you got suggests that it didn't fill in the port number from the config file for forwarding host "jump", or maybe that it did fill it in both overrode it later back to the default, and this failed because there was no listener on the default port 22 on 127.0.0.3.

I think I may be on to something. When I changed my SSH config from:

Host jump
    Hostname localhost
    Port 2222

Match final host ultra
    ProxyJump jump

to:

Match final Host jump
    Hostname localhost
    Port 2222

Match final host ultra
    ProxyJump jump

...it properly set the port number on my jump host to 2222. However, when I left out the "Match final" on the jump host but set it on the block that sets ProxyJump, the port number for the jump host defaulted to 22. Somehow the port number of the "Host jump" rule isn't matching when the ProxyJump is set in a "Match final". This is kind of odd, as the Hostname is still getting evaluated in this case. Otherwise, it'd error out on the nonexistent host named "jump".

I'll continue to dig into this and let you know what I find.

Ok - I think I see the problem. When there's a "match final" block present in the config, the config is reloaded after canonicalizing the host:

    host = canon_host if canon_host else options.host
    canonical = bool(canon_host)
    final = options.config.has_match_final()

    if canonical or final:
        options.update(host=host, reload=True, canonical=canonical, final=final)

When this happens, though, the hostname has changed from 'jump' to 'localhost' in my test, causing it to no longer match on the port number set in the "Host jump" block, and replacing it with the default port.

One potential fix here would be to carry forward the port from the previous evaluation (ONLY in this reload case). If the port is overridden again because of the new hostname, that could still happen, but if the port is set on the host alias for the jump host and it sets both Hostname and Port, both of those could carry over to the reload as initial values.

Unfortunately, it's more complicated than this. If you try to do a "Match final host jump" instead of just "Host jump", the first evaluation sets the port to 22 since the match block doesn't match. Later, when reloaded with final=True, the block match succeeded, but by then the port number is set to 22, not () and this takes precedence over the "Port" config directive for the jump host.

I'll need to think about this a bit more.

As a workaround, I think you could move the "Port" and "User" directives from "Host jump" to "Host 127.0.0.3", as long as 127.0.0.3 is only ever used as a jump host. If you want to use multiple port numbers on that host, it would get a little more complicated, but you might even be able to do that with if you use "Match" on both the host and port.

I'm curious if setting Hostname and Port for a jump host works in the OpenSSH case even with "Match final" present in the config. Ideally, I'd want AsyncSSH to end up with the same behavior (unless OpenSSH also has a bug in this case).

I stumbled upon this as my asyncssh code & ~/.ssh/config does not work for somebody else.
asyncssh does not pick up the username from the jump host.

It works with OpenSSH.

I just gave my own final implementation a shot - it works (for this case).

INFO:asyncssh:Opening SSH connection to 127.0.0.3, port 58493
…
DEBUG:asyncssh:[conn=0] Received disconnect: Certificate principal mismatch for host 127.0.0.3 (9)

https://github.com/ronf/asyncssh/pull/721/changes

You left in some debugging statements in test_forward.py in your latest commit, and the change in connection.py to set reload to False should also be unnecessary. Does your change work without these?

It looks like the main result of your change is that the host value from the original load of the config is preserved when doing the later call with reload=True. which in most cases would cause it to match on the same blocks as it did the first time, so things like Port are set to the same value after the reload as it was before, when opening tunnels, rather than passing in the rewritten host when a "Hostname" directive matched in the initial load. I can see how that would help make sure things like the Port still gets set when it is in a Host block looking for the original name.

One other issue is that your change right now relies on accessing a private member of SSHConfig. I'm going to want to find a way to do that which holds onto the original host value in the connect code. Also, I'd prefer not to explicitly test if the config is an SSHServerConfig or not. You had to do that because _orig_host only exists on SSHClientConfig, but if I do this in the calling code I think I can avoid that check.

I appreciate your work here - you've definitely helped me narrow down the source of the issue. Now I just need to find a clean way to fix it.

Hi,

it works.

Ran 1729 tests in 254.254s

OK (skipped=88)

Ok - I've got a potential fix for this. It also updates the "canonicalize hostname" feature to always use the original hostname provided by the user, which better aligns with the OpenSSH documentation on that feature. If enabled, canonicalization is done on the hostname passed in and THAT hostname is then match against the configuration and possibly remapped to other names using the 'Hostname' directive. Previously, I was passing an already mapped hostname into the canonicalize call, which is not correct.

Similarly, if there's a re-evaluation due to "Match final" and canonicalization didn't apply, the original hostname is passed info the config when it is reloaded, rather than an already-mapped name.

Could you give the following a try?

diff --git a/asyncssh/connection.py b/asyncssh/connection.py
index aa56b59..caf5004 100644
--- a/asyncssh/connection.py
+++ b/asyncssh/connection.py
@@ -282,7 +282,7 @@ async def _canonicalize_host(loop: asyncio.AbstractEventLoop,
                              options: 'SSHConnectionOptions') -> Optional[str]:
     """Canonicalize a host name"""

-    host = options.host
+    host = options.orig_host

     if not options.canonicalize_hostname or not options.canonical_domains:
         logger.info('Host canonicalization disabled')
@@ -474,7 +474,7 @@ async def _connect(options: _Options, config: DefTuple[ConfigPaths],

     canon_host = await _canonicalize_host(loop, options)

-    host = canon_host if canon_host else options.host
+    host = canon_host if canon_host else options.orig_host
     canonical = bool(canon_host)
     final = options.config.has_match_final()

@@ -7295,6 +7295,7 @@ class SSHConnectionOptions(Options, Generic[_Options]):
     waiter: Optional[asyncio.Future]
     protocol_factory: _ProtocolFactory
     version: bytes
+    orig_host: str
     host: str
     port: int
     tunnel: object
@@ -7387,6 +7388,7 @@ class SSHConnectionOptions(Options, Generic[_Options]):
         self.protocol_factory = protocol_factory
         self.version = _validate_version(version)

+        self.orig_host = host
         self.host = cast(str, config.get('Hostname', host))
         self.port = cast(int, port if port != () else
             config.get('Port', DEFAULT_PORT))

Sure

Ran 1727 tests in 177.904s

OK (skipped=88)